Add panel-review command with multi-specialist code review panel#436
Open
stbenjam wants to merge 2 commits into
Open
Add panel-review command with multi-specialist code review panel#436stbenjam wants to merge 2 commits into
stbenjam wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Contributor
|
Caution Review failedFailed to post review comments WalkthroughAdds a new /code-review:panel-review command and a review-panel skill to the code-review plugin. Introduces flags --serial, --comment, and --coderabbit (flags listed before the PR identifier), extensive docs and a verdict template, and bumps the plugin version to 0.0.10. ChangesReview Panel feature
Sequence Diagram(s)sequenceDiagram
actor User
participant Command as "panel-review\nCommand"
participant Orchestrator as "Review\nOrchestrator"
participant Specialists as "Specialist\nSub-agents"
participant External as "External\nReviewers"
participant Arbiter as "Panel\nArbiter"
participant Template as "Verdict\nTemplate"
User->>Command: /code-review:panel-review [PR] [--serial] [--comment] [--coderabbit]
activate Command
Command->>Orchestrator: Dispatch with PR diff & params
deactivate Command
activate Orchestrator
par Parallel Specialist Review
Orchestrator->>Specialists: Dispatch 7 specialists
activate Specialists
Specialists-->>Orchestrator: Structured findings
deactivate Specialists
and Parallel External Review
Orchestrator->>External: Invoke external reviewer(s) (non-blocking)
activate External
External-->>Orchestrator: Tool output (optional)
deactivate External
end
Orchestrator->>Arbiter: Aggregate findings + external output
activate Arbiter
Arbiter-->>Orchestrator: Disposition + rationale
deactivate Arbiter
Orchestrator->>Template: Render verdict
activate Template
Template-->>Orchestrator: Rendered verdict
deactivate Template
Orchestrator-->>User: Verdict with panel disposition
deactivate Orchestrator
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/code-review/commands/panel-review.md`:
- Line 3: The argument-hint currently hardcodes "coderabbit" which conflicts
with the generic repeatable external reviewer contract used elsewhere; update
the argument-hint value (the "argument-hint" entry) to use the canonical
repeatable external-reviewer signature (e.g., "[pr-url-or-number]
[external-reviewer...]" or similar) so it matches the descriptions referenced at
lines 11 and 76 and any other occurrences of external-reviewer in this command
spec.
In `@plugins/code-review/skills/review-panel/SKILL.md`:
- Around line 214-218: Update the wording so it matches the verdict template:
locate the bullet that begins "External reviewers are **not** sub-agents." and
change the phrase that says their stdout "is captured as-is and included in the
verdict under its own heading in the specialist findings section" to instead say
it is included under the "External Reviewers" section; keep the other bullets
(e.g., the Panel Arbiter treating external reviewer output as a peer) intact but
ensure any references to "specialist findings" are replaced with "External
Reviewers" to avoid formatting drift.
- Around line 271-273: The shell snippet currently shows an unquoted placeholder
`<base-ref>` which risks argument-splitting or injection; update the example
command (the `timeout 300 coderabbit review --agent --base <base-ref> 2>&1`
line) to quote the placeholder (e.g. `--base "<base-ref>"`) so any substituted
value with spaces or special characters is treated as a single argument and
avoids shell injection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 592ab4b2-718f-4814-85cb-79c6a0927ad5
📒 Files selected for processing (6)
PLUGINS.mddocs/data.jsonplugins/code-review/README.mdplugins/code-review/commands/panel-review.mdplugins/code-review/skills/review-panel/SKILL.mdplugins/code-review/skills/review-panel/verdict-template.md
Comment on lines
+214
to
+218
| - External reviewers are **not** sub-agents. They are CLI commands | ||
| invoked via Bash, running in parallel with the sub-agent dispatch. | ||
| - Their stdout is captured as-is and included in the verdict under | ||
| its own heading in the specialist findings section. | ||
| - The Panel Arbiter treats external reviewer output as a **peer |
Contributor
There was a problem hiding this comment.
Align section wording with the verdict template.
Line 217 says external output goes into the specialist findings section, but the template defines a separate External Reviewers section. Please align this wording to avoid formatting drift.
✏️ Suggested wording update
-- Their stdout is captured as-is and included in the verdict under
- its own heading in the specialist findings section.
+- Their stdout is captured as-is and included in the verdict under
+ the **External Reviewers** section with its own heading.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - External reviewers are **not** sub-agents. They are CLI commands | |
| invoked via Bash, running in parallel with the sub-agent dispatch. | |
| - Their stdout is captured as-is and included in the verdict under | |
| its own heading in the specialist findings section. | |
| - The Panel Arbiter treats external reviewer output as a **peer | |
| - External reviewers are **not** sub-agents. They are CLI commands | |
| invoked via Bash, running in parallel with the sub-agent dispatch. | |
| - Their stdout is captured as-is and included in the verdict under | |
| the **External Reviewers** section with its own heading. | |
| - The Panel Arbiter treats external reviewer output as a **peer |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/code-review/skills/review-panel/SKILL.md` around lines 214 - 218,
Update the wording so it matches the verdict template: locate the bullet that
begins "External reviewers are **not** sub-agents." and change the phrase that
says their stdout "is captured as-is and included in the verdict under its own
heading in the specialist findings section" to instead say it is included under
the "External Reviewers" section; keep the other bullets (e.g., the Panel
Arbiter treating external reviewer output as a peer) intact but ensure any
references to "specialist findings" are replaced with "External Reviewers" to
avoid formatting drift.
stbenjam
force-pushed
the
panel-review-command
branch
from
e761f41 to
e764c4b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
plugins/code-review/commands/panel-review.md (1)
3-3:⚠️ Potential issue | 🟡 MinorUse one canonical argument signature in frontmatter.
argument-hintnarrows tocoderabbitwhile the command contract elsewhere is repeatable external reviewers.Suggested fix
-argument-hint: "[pr-url-or-number] [coderabbit]" +argument-hint: "[pr-url-or-number] [external-reviewer...]"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/code-review/commands/panel-review.md` at line 3, The frontmatter key argument-hint currently uses a non-canonical signature ("[pr-url-or-number] [coderabbit]") which conflicts with the command's contract that accepts repeatable external reviewers; update the argument-hint to the canonical, repeatable form to match the contract (e.g., "[pr-url-or-number] [external-reviewer...]" or similar), ensuring the frontmatter argument-hint exactly mirrors the command's contract for repeatable external reviewers so callers and docs remain consistent.plugins/code-review/skills/review-panel/SKILL.md (2)
208-210:⚠️ Potential issue | 🟠 MajorQuote
<base-ref>in command examples to avoid unsafe shell patterns.Both the invocation table and bash snippet leave
<base-ref>unquoted.Suggested fix
-| CodeRabbit | `coderabbit` | `coderabbit review --agent --base <base-ref>` | User passes `coderabbit` as argument; tool must be on PATH | +| CodeRabbit | `coderabbit` | `coderabbit review --agent --base "<base-ref>"` | User passes `coderabbit` as argument; tool must be on PATH | ```bash -timeout 300 coderabbit review --agent --base <base-ref> 2>&1 +timeout 300 coderabbit review --agent --base "<base-ref>" 2>&1</details> Also applies to: 271-273 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/code-review/skills/review-panel/SKILL.mdaround lines 208 - 210,
Update the CodeRabbit invocation examples to quote the placeholder so shell
globbing and word-splitting are avoided: change the invocation cell for the
CodeRabbit row (the CLI Tool "CodeRabbit" / invocationcoderabbit review --agent --base <base-ref>) to use--base "<base-ref>", and update the bash
example(s) that usetimeout 300 coderabbit review --agent --base <base-ref> 2>&1totimeout 300 coderabbit review --agent --base "<base-ref>" 2>&1; make
the same change for the duplicate occurrences mentioned (lines ~271-273).</details> --- `216-217`: _⚠️ Potential issue_ | _🟡 Minor_ **Align external output placement with the verdict contract.** This wording still points to the specialist findings section, but the template has a dedicated **External Reviewers** section. <details> <summary>Suggested fix</summary> ```diff -- Their stdout is captured as-is and included in the verdict under - its own heading in the specialist findings section. +- Their stdout is captured as-is and included in the verdict under + its own heading in the **External Reviewers** section.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/code-review/skills/review-panel/SKILL.md` around lines 216 - 217, Update the sentence "Their stdout is captured as-is and included in the verdict under its own heading in the specialist findings section." to reference the correct location in the verdict contract by replacing "specialist findings section" with "External Reviewers section" (or similar wording matching the template), so it reads that the stdout is captured as-is and included under the "External Reviewers" section of the verdict; ensure the phrase "Their stdout is captured as-is" is preserved and that the term "External Reviewers" matches the template wording used elsewhere in SKILL.md.
🧹 Nitpick comments (1)
plugins/code-review/README.md (1)
15-20: Consider aligning README usage with the repeatable external-reviewer contract.This keeps docs consistent with the command spec and avoids future drift as more external reviewers are added.
Suggested update
-/code-review:panel-review [pr-url-or-number] [coderabbit] +/code-review:panel-review [pr-url-or-number] [external-reviewer...] -- External reviewers (optional): Names of external review tools to include. Currently supported: `coderabbit`. +- External reviewers (optional, repeatable): Names of external review tools to include. Currently supported: `coderabbit`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/code-review/README.md` around lines 15 - 20, Update the README usage string and Arguments section to match the repeatable external-reviewer contract: change the example command "/code-review:panel-review [pr-url-or-number] [coderabbit]" to indicate that external reviewers are repeatable (e.g., "/code-review:panel-review [pr-url-or-number] [external-reviewer...]" or similar) and update the Arguments bullet to state that External reviewers can be listed multiple times and follow the "external-reviewer" contract (currently includes coderabbit). Ensure references to "PR identifier" and "External reviewers" mention the accepted formats and the repeatable nature so docs remain consistent with the command spec.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/code-review/commands/panel-review.md`:
- Around line 10-12: Replace unlabeled fenced code blocks in
plugins/code-review/commands/panel-review.md for the command examples with
language-labeled fences to satisfy MD040; specifically update each fenced block
that shows the command usage (examples of `/code-review:panel-review
[pr-url-or-number] [external-reviewer...]`, `/code-review:panel-review`,
`/code-review:panel-review coderabbit`, `/code-review:panel-review
https://github.com/openshift/hypershift/pull/789 coderabbit`,
`/code-review:panel-review 456`) to use ```text fences instead of plain ``` so
the examples are explicitly marked as text.
---
Duplicate comments:
In `@plugins/code-review/commands/panel-review.md`:
- Line 3: The frontmatter key argument-hint currently uses a non-canonical
signature ("[pr-url-or-number] [coderabbit]") which conflicts with the command's
contract that accepts repeatable external reviewers; update the argument-hint to
the canonical, repeatable form to match the contract (e.g., "[pr-url-or-number]
[external-reviewer...]" or similar), ensuring the frontmatter argument-hint
exactly mirrors the command's contract for repeatable external reviewers so
callers and docs remain consistent.
In `@plugins/code-review/skills/review-panel/SKILL.md`:
- Around line 208-210: Update the CodeRabbit invocation examples to quote the
placeholder so shell globbing and word-splitting are avoided: change the
invocation cell for the CodeRabbit row (the CLI Tool "CodeRabbit" / invocation
`coderabbit review --agent --base <base-ref>`) to use `--base "<base-ref>"`, and
update the bash example(s) that use `timeout 300 coderabbit review --agent
--base <base-ref> 2>&1` to `timeout 300 coderabbit review --agent --base
"<base-ref>" 2>&1`; make the same change for the duplicate occurrences mentioned
(lines ~271-273).
- Around line 216-217: Update the sentence "Their stdout is captured as-is and
included in the verdict under its own heading in the specialist findings
section." to reference the correct location in the verdict contract by replacing
"specialist findings section" with "External Reviewers section" (or similar
wording matching the template), so it reads that the stdout is captured as-is
and included under the "External Reviewers" section of the verdict; ensure the
phrase "Their stdout is captured as-is" is preserved and that the term "External
Reviewers" matches the template wording used elsewhere in SKILL.md.
---
Nitpick comments:
In `@plugins/code-review/README.md`:
- Around line 15-20: Update the README usage string and Arguments section to
match the repeatable external-reviewer contract: change the example command
"/code-review:panel-review [pr-url-or-number] [coderabbit]" to indicate that
external reviewers are repeatable (e.g., "/code-review:panel-review
[pr-url-or-number] [external-reviewer...]" or similar) and update the Arguments
bullet to state that External reviewers can be listed multiple times and follow
the "external-reviewer" contract (currently includes coderabbit). Ensure
references to "PR identifier" and "External reviewers" mention the accepted
formats and the repeatable nature so docs remain consistent with the command
spec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6ce2b518-0561-4d9f-b3b8-ea135ad976ea
📒 Files selected for processing (8)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/code-review/.claude-plugin/plugin.jsonplugins/code-review/README.mdplugins/code-review/commands/panel-review.mdplugins/code-review/skills/review-panel/SKILL.mdplugins/code-review/skills/review-panel/verdict-template.md
✅ Files skipped from review due to trivial changes (3)
- plugins/code-review/.claude-plugin/plugin.json
- plugins/code-review/skills/review-panel/verdict-template.md
- .claude-plugin/marketplace.json
🚧 Files skipped from review as they are similar to previous changes (2)
- PLUGINS.md
- docs/data.json
Seven specialist reviewers execute in parallel as sub-agents, each examining changes through a different lens: Architecture, Security & Supply Chain, UX/API, Codebase Consistency, QA Engineer, Devil's Advocate, and Technical Writer. A Panel Arbiter synthesizes findings, resolves disagreements, and produces a single verdict. Inspired by Microsoft's APM Review Panel. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
stbenjam
force-pushed
the
panel-review-command
branch
from
e764c4b to
9cd7f77
Compare
…n to panel-review - --serial: run all 7 specialists inline instead of as parallel sub-agents, sharing codebase context for significantly cheaper reviews - --comment: post the verdict as a PR comment (requires PR identifier) - --coderabbit: converted from positional keyword to flag for consistency - Prior review detection: when reviewing a PR, search for previous panel review comments and pass them as context so specialists can track addressed/unresolved findings and flag regressions - Verdict template now includes a footer link to the code-review plugin, which also serves as the marker for prior review detection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
stbenjam
force-pushed
the
panel-review-command
branch
from
9cd7f77 to
11abdac
Compare
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Seven specialist reviewers each examine changes through a different lens: Architecture, Security & Supply Chain, UX/API, Codebase Consistency, QA Engineer, Devil's Advocate, and Technical Writer. A Panel Arbiter synthesizes findings, resolves disagreements, and produces a single verdict.
Inspired by Microsoft's APM Review Panel.
New in this update
--serialmode: Runs all specialists inline in the main agent instead of as parallel sub-agents. Significantly cheaper since codebase context is derived once and shared across all specialists.--commentflag: Posts the verdict as a comment on the PR (requires a PR identifier).--coderabbitflag: Converted from positional keyword to--flagstyle for consistency.Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
New Features
Documentation
Chores